Skip to content

Match LIFT list values ignoring surrounding whitespace - #1200

Merged
imnasnainaec merged 5 commits into
mainfrom
lift-import-trim-possibility-names
Oct 29, 2026
Merged

imnasnainaec merged 5 commits into
mainfrom
lift-import-trim-possibility-names

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

FieldWorks semantic domain templates before 2017 have trailing spaces in 16 English names ("Necessary ", "Animism ", ...), and projects created then still have them. The Combine writes the trimmed 9.4.2.3 Necessary to LIFT (sillsdev/TheCombine#4410). That misses FLEx's key 9.4.2.3 Necessary , so import adds a duplicate domain.

 TryGetPossibilityMatchingTrait(value)
-  try value, lower(value)
+  TryGetPossibilityMatchingUntrimmed(value)     // value, lower(value)
+  else TryGetPossibilityMatchingTrimmed(value)  // trimmed, lower(trimmed);
+                                                // else scan for key.Trim() == trimmed

-FindComplexFormType(old entry-type) / FindVariantType(old entry-type)
+FindOldEntryType(old entry-type)
+  untrimmed complex form type, else untrimmed variant type,
+  else trimmed complex form type, else trimmed variant type

The maps are unchanged, so an untrimmed match always wins. The scan runs only after every direct lookup misses; hits aren't cached, so a value that matches only after trimming rescans on each occurrence.

Evidence

New tests in LiftMergerTests cover padded names in semantic domains, parts of speech and both entry-type lists, the precedence rules above, whitespace variants within one file, minor-entry-condition subtypes, and the no-match fallbacks.

  • Before: ImportSemanticDomainTrait_MatchesExistingDomainWithPaddedName on main: Should not create a new semantic domain. Expected: property Count equal to 1 But was: 2
  • After: LiftMergerTests and LiftMergerRelationTests: 79 passed, 0 failed, 2 skipped (TestLiftImport9C/9D, which skip themselves).

Merge Danger

Door: two-way. Import-time matching only; no model or data migration.

Blast Radius: LIFT import. The trimmed fallback applies to every list matched through TryGetPossibilityMatchingTrait (semantic domains, parts of speech, entry types, anthropology codes, dialects, usage types, locations and the rest), not just semantic domains.

Out of scope, still matched without trimming: reversal parts of speech, lexical relation types, exception features, morph types, list-valued custom fields, minor-entry-condition subtypes, and the old entry-type special cases (Derivation, main entry, ...).

Considered and rejected

  • Storing trimmed aliases in the possibility maps. An alias could claim a key before a later item whose name matched exactly.
  • Matching lift-ranges semantic domains by label. ProcessSemanticDomain matches range elements only by guid. That's separate, pre-existing behavior.

🤖 Generated with Claude Code


This change is Reviewable

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   10m 56s ⏱️ - 3m 19s
6 884 tests +16  6 799 ✅ +16  85 💤 ±0  0 ❌ ±0 
6 893 runs  +16  6 808 ✅ +16  85 💤 ±0  0 ❌ ±0 

Results for commit 9b9ace1. ± Comparison against base commit 5cbbfe8.

♻️ This comment has been updated with latest results.

@codecov-commenter

codecov-commenter commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.12%. Comparing base (5cbbfe8) to head (9b9ace1).

Files with missing lines Patch % Lines
Src/LexText/LexTextControls/LiftMergerRanges.cs 93.75% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1200      +/-   ##
==========================================
+ Coverage   40.09%   40.12%   +0.02%     
==========================================
  Files        1544     1544              
  Lines      355309   355328      +19     
  Branches    41357    41364       +7     
==========================================
+ Hits       142475   142574      +99     
+ Misses     183533   183468      -65     
+ Partials    29301    29286      -15     
Files with missing lines Coverage Δ
...LexTextControls/LiftMergerSupportCodeAndClasses.cs 59.98% <100.00%> (+2.12%) ⬆️
Src/LexText/LexTextControls/LiftMergerRanges.cs 75.04% <93.75%> (+2.49%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

imnasnainaec and others added 4 commits October 28, 2026 11:46
LIFT import matched list items, such as semantic domains, only on exact
text. Semantic domain lists from FieldWorks templates before 2017 have
trailing spaces in some English names (e.g., "Necessary "), so a LIFT
value of "9.4.2.3 Necessary" from another program created a duplicate
domain instead of linking to the existing one.

Also key possibility lookups on names and abbreviations trimmed of
surrounding whitespace, and fall back to the trimmed value when looking
up a trait.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Storing trimmed aliases in the possibility maps let an alias claim a
key before an item whose name matches exactly, and items created during
import got no alias, so whitespace variants within one file still
created duplicates.

Keep the maps as built from the project, and only when the exact and
lowercase lookups miss, scan for a key that matches once trimmed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolving the old entry-type trait looked up complex form and variant
types by exact or lowercase text only, so an existing variant type
"Foo " was missed and a new complex form type "Foo" was created. Use
the same lookup as the other LIFT trait values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolving an old entry-type value checked complex form types first,
including a trimmed match, so a complex form type "Foo " beat a variant
type that matched "Foo" exactly. Try exact matches on both lists before
trimmed matches on either; complex form types still win at equal
strength.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@imnasnainaec
imnasnainaec force-pushed the lift-import-trim-possibility-names branch from 13023b8 to 285bea5 Compare October 28, 2026 15:50
When a padded trait value such as "Foo " has no untrimmed match,
look up the trimmed value directly before scanning the map. A key
with no surrounding whitespace then wins over other keys that only
match once trimmed, whatever order they were added in.

Rename TryGetPossibilityMatchingExactly to
TryGetPossibilityMatchingUntrimmed, since it also matches the
lowercase form.

Add tests for the complex form side of old entry-type lookup, the
tie between complex form and variant types, minor-entry-condition
subtypes, the no-match fallbacks, case-insensitive trimmed matching
and padded part-of-speech names. Group the tests by behavior, name
them with "Padded" for stray whitespace, and give the semantic
domain senses fixed ids.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@imnasnainaec
imnasnainaec marked this pull request as ready for review October 28, 2026 16:49

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imnasnainaec
imnasnainaec merged commit 002280d into main Oct 29, 2026
8 of 9 checks passed
@imnasnainaec
imnasnainaec deleted the lift-import-trim-possibility-names branch October 29, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants